Auto report unable to decrypt errors via lab option by BillCarsonFr · Pull Request #4325 · element-hq/element-android

BillCarsonFr · 2021-10-25T08:33:43Z

Adds a new lab option to enable auto reporting of fail to decrypt errors. When detected the device will send a rageshake and then send a toDevice message to the other participant to request him to also upload a RS.

The AutoRageShake and UISIs detection part has been put in the app side, and in order to do that I added to new services in the SDK:

SendToDeviceService: A service that allows to send to device messages (needed by AutoRageshaker in order to ask for the matching rageshake)
EventStreamService: Provides entry points needed by the UISI detector (could not be done by listening at the db events..)

@ganfra / @bmarty If you could have a quick look on the SDK part and give some feedbaks

Sample report
https://github.com/matrix-org/element-android-rageshakes/issues/27579

github-actions · 2021-10-25T09:02:11Z

Unit Test Results

  66 files ±0   66 suites ±0 52s ⏱️ -1s
135 tests ±0 135 ✔️ ±0 0 💤 ±0 0 ❌ ±0
418 runs ±0 418 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit a047bcb. ± Comparison against base commit 32f2e7d.

♻️ This comment has been updated with latest results.

bmarty · 2021-11-19T09:12:55Z

@ganfra / @bmarty If you could have a quick look on the SDK part and give some feedbaks

Will do, I was waiting for the PR to not be draft to just read the description :/

bmarty

Some small first remarks.
Weird to expose a ToDeviceService, but why not...

bmarty · 2021-12-13T22:40:19Z

...-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultToDeviceService.kt

+import javax.inject.Inject
+
+internal class DefaultToDeviceService @Inject constructor(
+        private val sendToDeviceTask: DefaultSendToDeviceTask,


The type must be SendToDeviceTask

bmarty · 2021-12-13T22:41:37Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionModule.kt

+    abstract fun bindToDeviceService(deviceService: DefaultToDeviceService): ToDeviceService
+
+    @Binds
+    abstract fun bindEventStreamService(deviceService: DefaultEventStreamService): EventStreamService


Could be grouped with the other services. Also rename param to service (at 2 places)

bmarty · 2021-12-13T22:41:59Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/StreamEventsManager.kt

+    private val listeners = mutableListOf<LiveEventListener>()
+
+    fun addLiveEventListener(listener: LiveEventListener) {
+        Timber.v("## VALR: addLiveEventListener")


use logger tag instead of VALR :)

:) Updated, just a temporary log that I forgot to remove

duxovni · 2022-01-04T16:50:11Z

vector/src/main/java/im/vector/app/AutoRageShaker.kt

+                                                "device_id" to target.senderDeviceId,
+                                                "user_id" to target.senderUserId,
+                                                "sender_key" to target.senderKey,
+                                                "matching_issue" to reportUrl


matching_issue is a confusing name for this field, I'd expect that to be a link to a github issue or something; how would you feel about something more like recipient_rageshake?

Sorry, didn't realize the rageshake server actually does create a github issue and responds with that; I'd had that disabled in my tests, and assumed we were looking at the URL of the submitted rageshake.

I renamed matching_issue to recipient_rageshake.
Also align with the Z-UISI name for the issue label

bmarty · 2022-01-07T13:37:01Z

vector/src/main/res/xml/vector_settings_labs.xml

-        android:defaultValue="false"
-        android:key="SETTINGS_LABS_USE_RESTRICTED_JOIN_RULE"
-        android:summary="@string/labs_use_restricted_join_rule_desc"
-        android:title="@string/labs_use_restricted_join_rule" />


This removal will need more cleanup in the code, and not really related to this PR.

Agreed will restore and creating separate PR

PR up here #4889

bmarty

LGTM

ganfra

Nice work! I don't see any issue like that. Performance wise, it should be ok I guess as long as there is not too many listeners on EventStreamService.
Other way of doing that is exposing Flows of event.
Maybe we should at least make them suspend methods? (LiveEventListener)

github-actions · 2022-01-10T08:48:57Z

Ktlint Results

👍 ✅ 👍

BillCarsonFr force-pushed the feature/bca/auto_uisi branch 2 times, most recently from 73b143d to f6dc2af Compare December 6, 2021 08:37

BillCarsonFr marked this pull request as ready for review December 6, 2021 10:43

duxovni mentioned this pull request Dec 8, 2021

Add labs flag to automatically rageshake on decryption errors matrix-org/matrix-react-sdk#7307

Merged

bmarty reviewed Dec 13, 2021

View reviewed changes

BillCarsonFr force-pushed the feature/bca/auto_uisi branch from 7323137 to cfa2f34 Compare December 17, 2021 15:43

BillCarsonFr force-pushed the feature/bca/auto_uisi branch 3 times, most recently from e9cf9e8 to e56bb1a Compare January 4, 2022 08:57

duxovni reviewed Jan 4, 2022

View reviewed changes

BillCarsonFr force-pushed the feature/bca/auto_uisi branch from 793ac6b to bbc7e4c Compare January 6, 2022 15:37

bmarty reviewed Jan 7, 2022

View reviewed changes

bmarty approved these changes Jan 7, 2022

View reviewed changes

bmarty requested a review from ganfra January 7, 2022 13:40

ganfra approved these changes Jan 7, 2022

View reviewed changes

BillCarsonFr added 10 commits January 10, 2022 09:41

New SDK Api for to device and event stream listener

6a34b99

Support for lab auto rageshake UISI

be119ea

Fix UISIS preference listener

cf026b2

use flow instead of reactivex

401e8e6

Simple rate limiting of RS sending

56dac76

Fix enabling was broken

aa89e72

code review

de08afd

Change autoUisi label + rename matching_issue

258d731

Update nb of enum classes

3f0122b

restore deprecated lab preference

a047bcb

BillCarsonFr force-pushed the feature/bca/auto_uisi branch from bbc7e4c to a047bcb Compare January 10, 2022 08:46

BillCarsonFr merged commit 30aae3f into develop Jan 10, 2022

BillCarsonFr deleted the feature/bca/auto_uisi branch January 10, 2022 13:55

Comments

Conversation

BillCarsonFr commented Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

bmarty commented Nov 19, 2021

Uh oh!

bmarty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmarty left a comment

Choose a reason for hiding this comment

Uh oh!

ganfra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 10, 2022

Ktlint Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BillCarsonFr commented Oct 25, 2021 •

edited

Loading

github-actions bot commented Oct 25, 2021 •

edited

Loading

ganfra left a comment •

edited

Loading